Fix date formatting on server for CSV export#29977
Conversation
tsullivan
added
review
v7.0.0
zDeprecated Feature:Reporting
|
Reviewers: Let me know if you need to see a test added in this change. I think adding a new integration test that hits the CSV generation URL and then pulls the completed job out of ES might be the best option for now. |
tsullivan
force-pushed
the
fix/reporting-csv-tz-ii
branch
from
134f6c5 to
031846a
Compare
💚 Build Succeeded |
joelgriffith
left a comment
joelgriffith
left a comment
There was a problem hiding this comment.
Found one stray console.log and a few questions (probably just my lack of understanding)
| } | ||
|
|
||
| if (date.isValid()) { | ||
| console.log({ date: date.format(this._memoizedPattern) }); |
| }); | ||
| } | ||
|
|
||
| getParamDefaults() { |
There was a problem hiding this comment.
I didn't see this used elsewhere... is it an interface that this class implements?
There was a problem hiding this comment.
Hm, not sure. I copied most of this file from src/legacy/core_plugins/kibana/common/field_formats/types/date.js. All the format types seem to have that method.
There was a problem hiding this comment.
Speaking of "copied from", there are unrelated differences in the new file: _memoizedConverter is declared in the constructor instead of in _convert
There was a problem hiding this comment.
Coolio! Thanks for the clarifaction
💚 Build Succeeded |
|
@joelgriffith ready for another look |
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
|
Nice, LGTM |
* Fix date formatting on server for CSV export * remove stray console.log * allow async to act in parallel * Log a warning when "Browser" is the timezone
* Fix date formatting on server for CSV export * remove stray console.log * allow async to act in parallel * Log a warning when "Browser" is the timezone
* Fix date formatting on server for CSV export * remove stray console.log * allow async to act in parallel * Log a warning when "Browser" is the timezone
* Fix date formatting on server for CSV export * remove stray console.log * allow async to act in parallel * Log a warning when "Browser" is the timezone
|
@tsullivan this is PR is failing in 6.6.1 BC1. Here is the regression bug: #31131 |
|
@bhavyarm Apologies! This incorrectly has the 6.6.1 label. It is not a regression, and is fixed in 7.0+ |
|
I fixed the version labels. The fix is for 7.0+ |
4 participants
Summary
Closes #29463
Re-try #29781
Summarize your PR. If it involves visual changes include a screenshot or gif.
Checklist